Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix integer overflow for scene size on 32bit systems #994

Merged
merged 5 commits into from
Dec 21, 2020

Conversation

ghost
Copy link

@ghost ghost commented Dec 14, 2020

Integers are 32bits on 32bit systems, for instance raspberry pis. This causes scene stats to overflow above 2GB and produce garbage numbers.

To output proper int64 we would need to manually set up a StatResultType model and matching resolver. Switching to float has the same effect however, and requires no boilerplate.

@ghost ghost marked this pull request as draft December 14, 2020 22:59
@ghost ghost marked this pull request as ready for review December 15, 2020 20:58
@ghost
Copy link
Author

ghost commented Dec 15, 2020

Turns out file sizes are broken on ingest so we have to wipe all file sizes with a migration.

Ideally the scenes table should be rebuilt with integer size column so we don't have to convert int=>string=>float, but rebuilding scenes is a nightmare and there shouldn't be any risk of overflow anymore.

@Tweeticoats
Copy link
Contributor

can you bump the schema to version 17 as #988 has updated the schema to 16.

@ghost ghost force-pushed the fix-scenesize-overflow branch from d8cd40c to d17e9fc Compare December 21, 2020 01:40
@WithoutPants WithoutPants added this to the Version 0.5.0 milestone Dec 21, 2020
@WithoutPants WithoutPants added the bug Something isn't working label Dec 21, 2020
@WithoutPants WithoutPants merged commit e84c923 into stashapp:develop Dec 21, 2020
Tweeticoats pushed a commit to Tweeticoats/stash that referenced this pull request Feb 1, 2021
* Fix integer overflow for scene size on 32bit systems
* Cast to double in sqlite to prevent potential overflow
* Add migration to reset scene sizes and scan logic to repopulate if empty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants